Fix eltype invalidation for SparsityPatternCSC#304
Conversation
SparsityPatternCSC{Ti} <: AbstractMatrix{Bool} but the custom
Base.eltype method was returning Ti (the index type) instead of Bool.
This caused 24,906 method invalidations when loading the package,
as it invalidated the backedge from Base.eltype(::AbstractArray).
Change the custom method to SparseArrays.indtype instead, since
that's what the type parameter Ti actually represents. The eltype
is now correctly inherited from AbstractMatrix{Bool}.
Co-Authored-By: Chris Rackauckas <[email protected]>
|
@ChrisRackauckas this is nonsense. |
|
Actually this is a Boolean matrix so it seems to make sense. Test failures seem unrelated, I'll fix them on main and review properly tomorrow |
|
@oscardssmith |
|
I'd argue you'd either need to do this or make it not an AbstractMatrix{Bool}. But the combination is 😅 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #304 +/- ##
=======================================
Coverage 99.39% 99.39%
=======================================
Files 20 20
Lines 2145 2145
=======================================
Hits 2132 2132
Misses 13 13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gdalle
left a comment
gdalle
left a comment
There was a problem hiding this comment.
Looks good to me, I'll merge and release if the tests pass
|
Fix incoming in release 0.4.24, undergoing registration atm |
4 participants
Summary
SparsityPatternCSC{Ti} <: AbstractMatrix{Bool}definesBase.eltypeto returnTi(the index type) instead ofBool, causing 24,906 method invalidations when loading the packageBase.eltypetoSparseArrays.indtype, sinceTiis the index type, not the element typeBoolfromAbstractMatrix{Bool}Context
When profiling the TTFX (Time-To-First-Execution) of ModelingToolkit's DAE pipeline,
SparseMatrixColoringswas identified as the #1 source of method invalidations in the entire dependency tree. Theeltype(::SparsityPatternCSC{T})method invalidatedBase.eltype(::AbstractArray), creating a cascade of 24,906 recompilations across all downstream packages.Changes
src/graph.jl: ChangedBase.eltype(::SparsityPatternCSC{T}) where {T} = TtoSparseArrays.indtype(::SparsityPatternCSC{T}) where {T} = Ttest/graph.jl: Updated test to checkeltypereturnsBooland addedindtypetest forIntTest plan
🤖 Generated with Claude Code